feat(aws-fsx): construct for NetApp ONTAP file system #42

badmintoncryer · 2024-07-20T13:52:33Z

Issue # (if applicable)

Closes #41.

Reason for this change

There is no L2 construct for creating FSx for NetApp ONTAP file system.

Description of changes

Add OntapFileSystem class
Add MaintenanceTime class
- However LusterMaintenanceTime class already exists in aws-cdk-lib.aws-fsx module , it seems not to be used in other file systems such as NetApp ONTAP. Therefore, I newly added MaintenanceTime class.

new ocf.aws_fsx.OntapFileSystem(stack, 'OntapMultiAzFileSystem', {
  vpc,
  vpcSubnets: vpc.privateSubnets,
  storageCapacityGiB: 5120,
  ontapConfiguration: {
    automaticBackupRetention: Duration.days(7),
    dailyAutomaticBackupStartTime: new aws_fsx.DailyAutomaticBackupStartTime({
      hour: 1,
      minute: 0,
    }),
    deploymentType: ocf.aws_fsx.OntapDeploymentType.MULTI_AZ_2,
    diskIops: 15360,
    endpointIpAddressRange: '192.168.39.0/24',
    fsxAdminPassword: 'fsxPassword1',
    haPairs: 1,
    preferredSubnet: vpc.privateSubnets[0],
    routeTables: [vpc.privateSubnets[0].routeTable, vpc.privateSubnets[1].routeTable],
    throughputCapacity: 384,
    weeklyMaintenanceStartTime: new ocf.aws_fsx.MaintenanceTime({
      day: aws_fsx.Weekday.SUNDAY,
      hour: 1,
      minute: 0,
    }),
  },
  removalPolicy: RemovalPolicy.DESTROY,
});

The CI job failed due to a conflict with the peer dependency of aws-cdk-lib.
I would like to set the peer dependency version to the newest one.

https://github.com/open-constructs/aws-cdk-library/actions/runs/10023311572/job/27704071917?pr=42

Description of how you validated changes

Added both unit and integ tests.

Checklist

My code adheres to the CONTRIBUTING GUIDE

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

go-to-k

Thanks for your great contribution!

The amount of changes is too large to look at all at once, so I have commented on the few superficial things that need to be fixed. I would like to look at the core later when I have more time, but it may take some time to look at everything.

If you have links to detailed documentation that we reviewers should have seen beforehand, or your design or implementation intentions, it would be helpful if you could include them in the PR description or code comments. This would improve the quality and speed of my review and may make it easier for other reviewers to participate in the review.

src/aws-fsx/ontap-file-system.ts

package.json

src/aws-fsx/README.md

src/aws-fsx/ontap-file-system.ts

src/aws-fsx/maintenance-time.ts

Co-authored-by: Kenta Goto (k.goto) <[email protected]> Signed-off-by: Kazuho Cryer-Shinozuka <[email protected]>

badmintoncryer · 2024-08-03T06:04:36Z

@go-to-k Thank you for your review! I've addressed most of your comments (some are still pending and will be updated later). I will update my PR description to include links to the documentation or other relevant information. Please bear with me for a moment.

src/aws-fsx/ontap-file-system.ts

badmintoncryer · 2024-08-08T14:27:50Z

I'll add comments later.

.projenrc.ts

src/aws-fsx/ontap-file-system.ts

go-to-k · 2024-09-12T15:06:10Z

Hi, @badmintoncryer

Thanks for the references! It's very helpful!

How are the revisions coming along? I will resume the review when you are ready. (I'm aware that you are still in the process of correcting the following and other points.)

#42 (comment)

badmintoncryer · 2024-09-13T09:56:15Z

@go-to-k I'm really sorry for forgetting this issue😓 I'll resolve it and get back to you later.

src/aws-fsx/ontap-file-system.ts

Co-authored-by: Kenta Goto (k.goto) <[email protected]> Signed-off-by: Kazuho Cryer-Shinozuka <[email protected]>

go-to-k · 2024-09-23T12:15:27Z

Could you resolve the conflicts?

src/aws-fsx/ontap-file-system.ts

badmintoncryer · 2024-10-01T14:33:45Z

@go-to-k
I was planning to reach out once everything had settled, but I appreciate your prompt response.

I've implemented the ThroughputCapacityPerHaPair class and added validation logic.

However, I’m not very confident about the overall approach, so I’d like your feedback. In particular, I’m not sure about the method name MB_PER_SEC_NUMBER, and I think it could be improved.

go-to-k · 2024-10-02T11:45:53Z

In particular, I’m not sure about the method name MB_PER_SEC_NUMBER, and I think it could be improved.

MB_PER_SEC_128
PER_SEC_128_MB
PER_HA_PAIR_128_MB
VALUE_128_MB_PER_SEC
SIZE_128_MB_PER_SEC
CAPACITY_128_MB_PER_SEC
THROUGHPUT_128_MB_PER_SEC
THROUGHPUT_CAPACITY_128_MB_PER_SEC
etc...

Hmmm... That's difficult... I prefer the format of XX_128_MB_PER_SEC or XX_128_MB, but MB_PER_SEC_128 is also good...

go-to-k · 2024-10-02T11:57:50Z

Please also change the README, JSDOC, and tests later (if we need to change anything to match your implementation).

go-to-k · 2024-10-02T12:08:50Z

Hmmm... That's difficult... I prefer the format of XX_128_MB_PER_SEC or XX_128_MB, but MB_PER_SEC_128 is also good...

Let's go with MB_PER_SEC_NUMBER for now!

go-to-k

Great changes, very good! I have made some comments. I will approve this PR if they are modified.

src/aws-fsx/ontap-file-system.ts

go-to-k

Could confirmed all changes. Thanks for this PR. I approve and merge.

badmintoncryer · 2024-10-05T07:18:32Z

@go-to-k Thank you very much for your review!

badmintoncryer added 6 commits July 20, 2024 22:01

add ontap file system

5b275d1

add unit test

fc99b09

add readme and tests

bb14369

remove temp dir

aa90d76

back

93863d8

add snap shot

17021fc

badmintoncryer mentioned this pull request Jul 20, 2024

feat(fsx): construct for NetApp ONTAP file system aws/aws-cdk#30849

Closed

1 task

badmintoncryer marked this pull request as ready for review July 25, 2024 12:34

go-to-k requested changes Jul 28, 2024

View reviewed changes

go-to-k reviewed Jul 28, 2024

View reviewed changes

src/aws-fsx/maintenance-time.ts Show resolved Hide resolved

badmintoncryer and others added 5 commits August 1, 2024 19:58

Update src/aws-fsx/ontap-file-system.ts

a6dd1e9

Co-authored-by: Kenta Goto (k.goto) <[email protected]> Signed-off-by: Kazuho Cryer-Shinozuka <[email protected]>

upgrade cdk via projen

286ae9d

add line breaks

2241345

add token varidation

50b5477

add default description

d475761

badmintoncryer commented Aug 8, 2024

View reviewed changes

mbonig reviewed Aug 8, 2024

View reviewed changes

.projenrc.ts Outdated Show resolved Hide resolved

badmintoncryer commented Aug 9, 2024

View reviewed changes

src/aws-fsx/ontap-file-system.ts Outdated Show resolved Hide resolved

badmintoncryer commented Aug 9, 2024

View reviewed changes

src/aws-fsx/ontap-file-system.ts Outdated Show resolved Hide resolved

badmintoncryer commented Aug 9, 2024

View reviewed changes

src/aws-fsx/ontap-file-system.ts Outdated Show resolved Hide resolved

badmintoncryer commented Aug 9, 2024

View reviewed changes

src/aws-fsx/ontap-file-system.ts Show resolved Hide resolved

go-to-k changed the title ~~feat(fsx): construct for NetApp ONTAP file system~~ feat(aws-fsx): construct for NetApp ONTAP file system Sep 14, 2024

update jsdoc

26c4181

go-to-k reviewed Sep 17, 2024

View reviewed changes

src/aws-fsx/ontap-file-system.ts Outdated Show resolved Hide resolved

Update src/aws-fsx/ontap-file-system.ts

1fa1694

Co-authored-by: Kenta Goto (k.goto) <[email protected]> Signed-off-by: Kazuho Cryer-Shinozuka <[email protected]>

badmintoncryer added 5 commits October 1, 2024 20:14

resolve conflicts

4815903

make deployment type optional

4efcf1c

update jsdoc

d73481c

define ThroughputCapacityPerHaPair

a0145c4

rename factory method

83a3637

go-to-k requested changes Oct 1, 2024

View reviewed changes

src/aws-fsx/ontap-file-system.ts Outdated Show resolved Hide resolved

go-to-k requested changes Oct 1, 2024

View reviewed changes

src/aws-fsx/ontap-file-system.ts Outdated Show resolved Hide resolved

badmintoncryer added 2 commits October 1, 2024 23:25

fix unit test

fe759cf

udpate jsdoc

6c639e8

badmintoncryer added 5 commits October 3, 2024 23:06

refactor

9c82af9

update unit test

5289299

rebuild

7100f64

update readme

f451f48

update integ

8a44f2c

go-to-k requested changes Oct 4, 2024

View reviewed changes

src/aws-fsx/ontap-file-system.ts Outdated Show resolved Hide resolved

src/aws-fsx/ontap-file-system.ts Outdated Show resolved Hide resolved

src/aws-fsx/ontap-file-system.ts Outdated Show resolved Hide resolved

src/aws-fsx/ontap-file-system.ts Outdated Show resolved Hide resolved

badmintoncryer added 4 commits October 5, 2024 12:05

remove unnecessary validation

89a0bfb

update

e28cd72

update api.md

1cf9ddb

remove allowedCapacity

9dedb3b

go-to-k approved these changes Oct 5, 2024

View reviewed changes

go-to-k added this pull request to the merge queue Oct 5, 2024

Merged via the queue into open-constructs:main with commit 3fe93f5 Oct 5, 2024
6 checks passed

badmintoncryer deleted the 41-ontap branch October 5, 2024 11:00

badmintoncryer mentioned this pull request Oct 8, 2024

(aws-fsx): NetApp ONTAP file system aws/aws-cdk#30848

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(aws-fsx): construct for NetApp ONTAP file system #42

feat(aws-fsx): construct for NetApp ONTAP file system #42

badmintoncryer commented Jul 20, 2024 •

edited

Loading

go-to-k left a comment

badmintoncryer commented Aug 3, 2024 •

edited

Loading

badmintoncryer commented Aug 8, 2024

go-to-k commented Sep 12, 2024

badmintoncryer commented Sep 13, 2024

go-to-k commented Sep 23, 2024

badmintoncryer commented Oct 1, 2024

go-to-k commented Oct 2, 2024 •

edited

Loading

go-to-k commented Oct 2, 2024

go-to-k commented Oct 2, 2024 •

edited

Loading

go-to-k left a comment

go-to-k left a comment

badmintoncryer commented Oct 5, 2024

feat(aws-fsx): construct for NetApp ONTAP file system #42

feat(aws-fsx): construct for NetApp ONTAP file system #42

Conversation

badmintoncryer commented Jul 20, 2024 • edited Loading

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

go-to-k left a comment

Choose a reason for hiding this comment

badmintoncryer commented Aug 3, 2024 • edited Loading

badmintoncryer commented Aug 8, 2024

go-to-k commented Sep 12, 2024

badmintoncryer commented Sep 13, 2024

go-to-k commented Sep 23, 2024

badmintoncryer commented Oct 1, 2024

go-to-k commented Oct 2, 2024 • edited Loading

go-to-k commented Oct 2, 2024

go-to-k commented Oct 2, 2024 • edited Loading

go-to-k left a comment

Choose a reason for hiding this comment

go-to-k left a comment

Choose a reason for hiding this comment

badmintoncryer commented Oct 5, 2024

badmintoncryer commented Jul 20, 2024 •

edited

Loading

badmintoncryer commented Aug 3, 2024 •

edited

Loading

go-to-k commented Oct 2, 2024 •

edited

Loading

go-to-k commented Oct 2, 2024 •

edited

Loading